[DPE-4683] Typed configuration #337
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #337 +/- ##
==========================================
+ Coverage 73.09% 73.35% +0.25%
==========================================
Files 8 9 +1
Lines 1286 1321 +35
Branches 224 227 +3
==========================================
+ Hits 940 969 +29
- Misses 267 272 +5
- Partials 79 80 +1 ☔ View full report in Codecov by Sentry. |
dragomirp
changed the title
dragomirp
changed the title
There was a problem hiding this comment.
Mostly copypaste from PG VM charm
| @@ -143,7 +143,7 @@ async def test_remove_tls(ops_test: OpsTest, pgb_charm_jammy): | |||
| @pytest.mark.group(1) | |||
| async def test_remove_vip(ops_test: OpsTest): | |||
| async with ops_test.fast_forward(): | |||
| await ops_test.model.applications[PGB].set_config({"vip": ""}) | |||
| await ops_test.model.applications[PGB].reset_config(["vip"]) | |||
There was a problem hiding this comment.
Empty string will no longer pass the address validation.
| try: | ||
| self._grafana_agent = COSAgentProvider( | ||
| self, | ||
| metrics_endpoints=[ | ||
| {"path": "/metrics", "port": self.config.metrics_port}, | ||
| ], | ||
| log_slots=[f"{PGBOUNCER_SNAP_NAME}:logs"], | ||
| refresh_events=[self.on.config_changed], | ||
| ) | ||
| except ValueError: | ||
| logger.warning("Unable to set COS agent, invalid config") |
There was a problem hiding this comment.
Data will be validated the first time we access self.config
| metrics_port: PositiveInt | ||
| vip: Optional[IPvAnyAddress] | ||
| pool_mode: Literal["session", "transaction", "statement"] | ||
| max_db_connections: conint(ge=0) |
There was a problem hiding this comment.
0 is unlimited db conns.
| if not self.charm.configuration_check(): | ||
| return |
There was a problem hiding this comment.
Function will be rerun on config change
| @@ -229,7 +227,7 @@ def _on_relation_broken(self, event: RelationBrokenEvent) -> None: | |||
|
|
|||
| def update_connection_info(self, relation): | |||
| """Updates client-facing relation information.""" | |||
| if not self.charm.unit.is_leader(): | |||
| if not self.charm.unit.is_leader() or not self.charm.configuration_check(): | |||
There was a problem hiding this comment.
Function will be rerun on config change.
| if not self.configuration_check(): | ||
| return | ||
|
|
There was a problem hiding this comment.
File will be rerendered on config change.
dragomirp
requested review from
a team,
taurus-forever,
marceloneppel and
lucasgameiroborges
and removed request for
a team
|
|
||
| # Make a directory for each service to store configs. | ||
| for service_id in self.service_ids: | ||
| os.makedirs(f"{app_conf_dir}/{INSTANCE_DIR}{service_id}", 0o700, exist_ok=True) | ||
| os.chown(f"{app_conf_dir}/{INSTANCE_DIR}{service_id}", pg_user.pw_uid, pg_user.pw_gid) | ||
| os.makedirs(f"{app_run_dir}/{INSTANCE_DIR}{service_id}", 0o777, exist_ok=True) |
There was a problem hiding this comment.
Should we limit it to read for others? Do they need a write capabilities?
There was a problem hiding this comment.
Clients could be other users, so we can't limit the reads for sure, they should be able to traverse to the socket. Writes we most probably don't need, but let's tweak it in the follow up PRs.
Boilerplate for #343